Thanks for this, @eclairevoyant, very much appreciated. Currently I'm a bit confused by the naming—not that there is anything wrong with it per se, could just be my current understanding of nixpkgs best practices.

The help clear this up, @eclairevoyant, could you provide more context or explanation around the motivation to call the "original"/"unmodified" package that offers no "customization" of ssh in got got-unwrapped and the one that does got?

Is it just because the "new" got is a wrapper around the "old" one?

It's how every other wrapped package I've seen is named. See nvim, mpv, sway, etc.

The more I go over this, the more I'm beginning to understand, and starting to really liking the change, @eclairevoyant, very well done!

Prior to review just a minor question more out of interest than anything else:
What is the motivation / pro / con for patching the source over using substituteInPlace in package.nix to replace execv(GOT_DIAL_PATH_SSH with execvp(GOT_DIAL_PATH_SSH in lib/dial.c?

I don't have a strong opinion either way; I can switch it to substituteInPlace if that's easier to maintain.

I think that substituteInPlace provides more flexibility if upstream changes the code around the execv calls; a patch is likely to fail just because some surrounding code changes, e.g. added or deleted lines in the source file that do not directly affect the execv call…

Didn't notice those earlier. I find that it's weird that they hardcode the editor at build-time instead of using an envvar with a default of vi or something.

Yeah nvm I think this is just the completely wrong approach

Thanks for the work on this, @eclairevoyant, too bad this doesn't seem to head in the right direction for you.
What would be a better approach? Is this something that is better raised and addressed upstream?

If they're just shelling out, this is something best handled at runtime via some envvar, not via buildtime #define. And yes upstream would be best to address this IMO

ℹ️ It seems the changes introducing GOT_DEFAULT_EDITOR were motivated by someone thinking of NixOS (see https://marc.gameoftrees.org/mail/1710762601.52948_0.html)

There's really no reason that they should require users to recompile got, just to use another editor. It's a bad design.

git does the sensible thing and reads VISUAL or EDITOR envvars by default.

got does indeed check and use the VISUAL and EDITOR envvars (see cvg/cvg.c:275f), only its default editor is hardcoded, which I believe make sense to change for nixpkgs if people so desire.

How about continuing the conversation over on #322600, @eclairevoyant?